polish(projects): streamline overview navigation and state - #6600
Conversation
Cassandra adversarial re-review — exact head
|
9381411 to
07d2ce6
Compare
Gauge review — PR #6600 at head
|
ef0d94a to
81ee2e0
Compare
Cassandra exact-head adversarial delta review —
|
Gauge delta review — PR #6600 rebuilt head
|
Make the Projects overview section-aware, searchable, responsive, and consistent with sidebar and empty-state patterns while keeping list rendering bounded. Signed-off-by: Thomas Petersen <thomasp@squareup.com> Co-authored-by: Thomas Petersen <thomasp@squareup.com>
81ee2e0 to
20dbe5a
Compare
Gauge delta review — PR #6600 replacement head
|
Cassandra replacement-head delta review —
|
Co-authored-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz> Signed-off-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz>
Gauge review #4 — CI triage at exact head
|
|
Cassandra exact-head CI follow-up for The completed job reports 7 hard failures, 2 flaky, 258 passed, 1 skipped. All five Part 6 failures previously identified by Gauge remain deterministic through both retries:
The two other hard failures match the inherited Part 5 failures:
The two tests that failed initially but passed on retry are outside the Part 6 Projects delta and remain classified as flakes:
The narrow stale-contract patch did what it claimed: the removed |
Gauge review #5 — delta review + CI at exact head
|
Co-authored-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz> Signed-off-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz>
|
Cassandra replacement-head review of P1 — repository-only data is restored in the lists but still excluded from the context rail (confidence 100)Evidence:
Concrete trigger: a community containing only a standalone kind-30617 repository with a bound channel now reaches the Projects surface, and the Repositories/Channels lists correctly show it after this replacement. However, the adjacent Repositories rail says The existing repository-only E2E checks list reachability/content, not these context totals or people, so the regression is not covered. The safe fix needs section-aware rail inputs: retain explicit projects for explicit-project counts/cards, while deriving repository/channel totals and repository activity people from all read models (or pass those aggregates explicitly). Merely changing the single Coverage with no additional findingsI reviewed the full five-file delta plus the surrounding work-item fan-out, channel grouping/collapse, profile lookup, people rendering, context derivation, standalone repository read-model construction, and the changed E2Es. The all-read-model work-item query itself is sound and deduplicates repository addresses; the restored people component does not introduce an injection boundary; the padding and orphan-assertion changes are scoped. The search assertion now exercises seeded data and both supported layouts, but is weaker than the prior exact row assertion; I found no correctness defect from that alone. Residual/gate: exact-head CI is still running and remains binding after this finding is fixed. The inherited Part 5 Smoke (3) failure must still be resolved in #6597 or explicitly adopted before a green stacked run is possible. |
Gauge review #6 — delta review at exact head
|
Co-authored-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz> Signed-off-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz>
|
Cassandra re-review of exact replacement head I reviewed the full
Independent focused verification at exact HEAD: Final gate remains fresh, fully green exact-head CI. Separately, the inherited #6597 commit-feed timeout still needs resolution in its owning branch or explicit adoption here before the stacked run can be called green. |
Gauge review #7 — delta review at exact head
|
Gauge review #8 — CI classification at exact head
|
Co-authored-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz> Signed-off-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz>
|
Cassandra delta review of exact head The delta from reviewed I traced the surrounding implementation rather than accepting the locator alone:
This does not weaken application coverage or hide the original timeout. It removes a keyboard/focus side effect unrelated to the test's stated commit-detail contract. No application code or security boundary changed. Diff check is clean and remote/local exact head matched during review. Fresh exact-head CI run |
Gauge review #9 — delta review at exact head
|
Gauge — CI classification, run 32678638549 @ exact head
|
| Job | Result |
|---|---|
| Smoke (1), (2), (4) | ✅ green |
| Desktop Core, macOS Build, Relay, Integration (1/2, 2/2) | ✅ green |
| Smoke (3) | ❌ 1 failed, 1 flaky |
✅ Confirmed fixed
project-commit-detail.spec.ts:734— passed in 5.2s (✓ 212). Wrench's Escape→Close-button fix is CI-confirmed. Confidence: 100.
❌ Hard failure — base-inherited, NOT caused by pt6
project-pr-review.spec.ts:1306"overview tasks and reviews are grouped and selectable by project" — failed all 3 attempts with an identical signature:Locator: getByTestId('projects-issue-project-group').first().locator('[data-testid^="projects-issue-row-"]') Expected: 30 Received: 56 at desktop/tests/e2e/project-pr-review.spec.ts:1347:24- Inheritance evidence (confidence 100): the pt5 base run at
07d2ce6ff(job 97273114460) shows the exact same failure —Expected: 30 / Received: 56at:1347— where it passed on retry Dependency Dashboard #1 (flaky bucket). It also flaked atf0bb9d37b(job 97287469655). The pt6 deltaf0bb9d37b..344bf64f1is 4 lines inproject-commit-detail.spec.tsonly, so it cannot influence this spec. - Root cause (confidence 75, quoted): the test snapshots the row count while data is still streaming, then asserts the same count after a collapse/expand round-trip:
(
const rowCount = await rows.count(); // captured at 30, group still loading ... await expect(rows).toHaveCount(0); // collapse await toggle.click(); await expect(rows).toHaveCount(rowCount); // group now fully loaded: 56 ≠ 30
project-pr-review.spec.ts~:1327–:1347). Same count-before-stable class as the previously adopted fixes. A stabilization fix: wait for the row count to settle (or for the overview loaded state) before capturingrowCount.
⚠️ Flaky (passed on retry, non-blocking)
overscroll-boundary.spec.ts:34"locks viewport rubber-band outside conversation scrollers" — new to the flake list for this PR.
Options
- Adopt a stabilization fix for
:1306(recommended — 3/3 deterministic failure here means a bare re-run is a coin flip at best; base evidence says it can pass on retry, but the failure rate is worsening). - Re-run Smoke (3) and hope the retry-pass behavior from the base run recurs — not recommended given 0/3 attempts passed in this run.
Strict gate holds: merge-ready requires fresh full-green CI at the exact head SHA, all shards including Smoke (3).
Co-authored-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz> Signed-off-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz>
|
Cassandra delta review of exact head I reviewed the complete two-file
The attribute is numeric React-owned metadata, not user-interpreted HTML or an authorization surface. It does not expose secrets, influence selection behavior, add sleeps/timeouts, or alter production rendering. It is also meaningful for every existing Diff check is clean; local and PR head matched during review. Fresh exact-head CI run |
Gauge — review #10, exact head
|
|
Cassandra final verdict for PR #6600: MERGE-READY at exact head I independently verified that PR head still equals the reviewed SHA and binding run No open security or correctness findings remain. The repository-only empty-state and context-scope P1s are closed with behavior/unit coverage; all Part 6 deterministic regressions are closed; the inherited Smoke (3) failures adopted here are now CI-confirmed. Residual UX/test-hardening notes already recorded in earlier durable review comments are non-blocking and do not change this verdict. |
Gauge — final CI classification & merge-ready verdict, run 32680631241 @ exact head
|
| Job | Result |
|---|---|
| Smoke (1)(2)(3)(4) | ✅ all green |
| Desktop Core, macOS Build, Relay E2E, Integration (1/2, 2/2) | ✅ green |
Smoke (3) detail (job 97296702656): 264 passed / 3 flaky / 1 skipped / 0 failed
project-pr-review.spec.ts:1306— ✓ passed first attempt in 3.3s (✓ 243). Thedata-project-group-sizesettled-count wait closed the 30-vs-56 incremental-mount race. Confidence: 100.project-commit-detail.spec.ts:734— ✓ passed first attempt in 5.4s (✓ 214). The Escape→Close fix holds across runs.- Flaky (passed on retry, all with prior history, non-blocking):
messaging.spec.ts:2049,overscroll-boundary.spec.ts:34,persistent-agent-audience.spec.ts:205.
Review-chain summary (pt6, every head reviewed)
07d2ce6ff (pt5 base) → 20dbe5a4f → de324a34a → 9d9593b65 → f0bb9d37b → 344bf64f1 → 6eac8f188. All pt6-caused findings (A1–A6, context-scope P1) closed and CI-confirmed; both adopted base-inherited stabilizations (:734, :1306) fixed at root cause and green under the full shard.
Residuals on record (non-blocking, already filed)
- App-side Escape double-dismiss (dialog + workspace sheet in one keypress) — routed around by the :734 fix, not resolved; predates this stack (comment 5389570320).
- One-shot
data-project-group-sizeread could theoretically go stale under a still-appending source [50] (comment 5389797154). - Collapsed-header
countuses the sliced group mid-mount [25] — pre-existing, cosmetic-transient (comment 5389797154).
No open findings against #6600's content. From the correctness/testing/reliability lens, this PR is clear to merge.
f0192ea
into
projects-channel-first-pt5-navigation-requests
## Summary - seed new project homes with a built-in operational canvas while preserving custom templates and None - make category create actions open the matching project-aware modal and keep selection presentation type-specific - aggregate Tasks and Commits across every repository with responsive project/repository grouping and a repository Overview tab This is the final part of the channel-first Projects stack, based on #6600. ## Testing - focused project template, work-item aggregation, and overview-context tests: 16/16 passed - Desktop unit suite: 5,455/5,455 passed - E2E-mode Desktop build passed - TypeScript, Biome, and differential file-size checks passed - full pre-push gate passed ## Post-Deploy Monitoring & Validation - create a project, inspect its canvas, and exercise Tasks/Commits across a project with several repositories - healthy signals: populated project canvas, one chronological commit list with repository attribution, and complete task aggregation - failure signals: missing canvas, single-repository-only counts, duplicate repository groups, or category actions opening the wrong flow; mitigate by reverting this PR --------- Signed-off-by: Thomas Petersen <thomasp@squareup.com> Signed-off-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz> Co-authored-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz>
## Summary - make the Projects overview follow the active section with consistent context, empty states, and row alignment - add an animated section-local search that preserves sort and responsive tab behavior - simplify sidebar and list controls while keeping large project collections incrementally mounted This is Part 6 of the channel-first Projects stack, based on #6597. The final part contains project workspace aggregation and the default project-home template. ## Testing - focused search, activity digest, agent selection, related-channel, and overview-context tests: 26/26 passed - Desktop unit suite: 5,451/5,451 passed - E2E-mode Desktop build passed - TypeScript, Biome, and differential file-size checks passed - full pre-push gate passed ## Post-Deploy Monitoring & Validation - exercise every Projects overview section, search transition, selection state, and right context rail in staging Desktop - healthy signals: stable tabs, aligned rows, correct section context, and responsive large-list interaction - failure signals: search flicker, stale context, wrapped toolbar rows, missing empty actions, or input lag; mitigate by reverting this PR --------- Signed-off-by: Thomas Petersen <thomasp@squareup.com> Signed-off-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz> Co-authored-by: Wrench <0eabe6ea5758c1e4c5b68cea4ac42b32c479072883cb28da8110b4a47c32b9a1@buzz.block.builderlab.xyz> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Summary
This is Part 6 of the channel-first Projects stack, based on #6597. The final part contains project workspace aggregation and the default project-home template.
Testing
Post-Deploy Monitoring & Validation